Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify output from make pyspec #3938

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Conversation

jtraglia
Copy link
Member

From this:

image

To this:

image

Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the clean up a lot!

left some comments but generally on board with these changes

@@ -394,8 +404,6 @@ def initialize_options(self):
def finalize_options(self):
"""Post-process options."""
if len(self.md_doc_paths) == 0:
print("no paths were specified, using default markdown file paths for pyspec"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this PR change anything if someone does provide non-default paths?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. I don't even know how to provide a non-default path. Is this something we do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if its used but it does seem like the code currently supports it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so it doesn't change any functionality with that. I just removed the print call.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't used the path flags, but Proto did when he designed it: https://github.com/ethereum/consensus-specs/tree/v1.5.0-alpha.6/tests/core/pyspec#advanced

Given that no current use cases require us to only build phase0 specs, I think it’s OK to ignore it for now.

Comment on lines +38 to +47
# Ignore '1.5.0-alpha.*' to '1.5.0a*' messages.
import warnings
warnings.filterwarnings('ignore', message='Normalizing .* to .*')

# Ignore 'running' and 'creating' messages
import logging
class PyspecFilter(logging.Filter):
def filter(self, record):
return not record.getMessage().startswith(('running ', 'creating '))
logging.getLogger().addFilter(PyspecFilter())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this stuff tends to be fragile so it could more easily break in the future

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah maybe. Shouldn't be difficult to update if something breaks. Worst case we can just delete it.

Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks refreshing. 😎

@@ -394,8 +404,6 @@ def initialize_options(self):
def finalize_options(self):
"""Post-process options."""
if len(self.md_doc_paths) == 0:
print("no paths were specified, using default markdown file paths for pyspec"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't used the path flags, but Proto did when he designed it: https://github.com/ethereum/consensus-specs/tree/v1.5.0-alpha.6/tests/core/pyspec#advanced

Given that no current use cases require us to only build phase0 specs, I think it’s OK to ignore it for now.

@jtraglia jtraglia merged commit 3162c91 into ethereum:dev Sep 26, 2024
26 checks passed
@jtraglia jtraglia deleted the cleaner-make-pyspec branch September 26, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants